-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add an explicit in-memory buffer #1217
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Size Change: -575 B (-0.06%) Total Size: 973 kB
ℹ️ View Unchanged
|
private _endpoint: string | ||
private flushBufferTimer?: any | ||
|
||
// we have a buffer - that contains PostHog snapshot events ready to be sent to the server | ||
private buffer?: SnapshotBuffer | ||
private buffer: SnapshotBuffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no benefit to this being allowed to be undefined
@@ -286,7 +309,7 @@ export class SessionRecording { | |||
this.sessionId = sessionId | |||
this.windowId = windowId | |||
|
|||
this.buffer = this.clearBuffer() | |||
this.buffer = new InMemoryBuffer(this.sessionId, this.windowId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explicitly assigning it so that it doesn't need to be undefinable
sessionId: this.sessionId, | ||
windowId: this.windowId, | ||
} | ||
private clearBuffer(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but keeping this convenience method for clarity
@@ -881,42 +891,31 @@ export class SessionRecording { | |||
this.flushBufferTimer = setTimeout(() => { | |||
this._flushBuffer() | |||
}, RECORDING_BUFFER_TIMEOUT) | |||
return this.buffer || this.clearBuffer() | |||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.buffer
can never be falsy anymore (and never would have been at this point anyway)
this._captureSnapshot({ | ||
$snapshot_bytes: this.buffer.size, | ||
$snapshot_data: this.buffer.data, | ||
$session_id: this.buffer.sessionId, | ||
$window_id: this.buffer.windowId, | ||
}) | ||
|
||
return this.clearBuffer() | ||
} else { | ||
return this.buffer || this.clearBuffer() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.buffer
never falsy anymore so no need for this branch
if (isNull(this.buffer.sessionId) && !isNull(this.sessionId)) { | ||
// session id starts null but has now been assigned, update the buffer | ||
this.buffer.sessionId = this.sessionId | ||
this.buffer.windowId = this.windowId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buffer always has a session id now
if someone has persistence set to memory only then we don't want to use local storage for replay
as a step towards a persistent queue for replay this moves buffering into an explicit in-memory buffer
clarifying the interface so that we can then add a persistent storage buffer